-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update common version and remove jersey code #538
Conversation
add on workflow_dispatch
# Conflicts: # .github/workflows/codeql-analysis.yml # hugegraph-client/src/main/java/org/apache/hugegraph/exception/ServerException.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement, hopefully we can get rid of jersey's issues from now on.
hugegraph-client/src/main/java/org/apache/hugegraph/api/auth/UserAPI.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/main/java/org/apache/hugegraph/api/graph/GraphAPI.java
Show resolved
Hide resolved
hugegraph-client/src/main/java/org/apache/hugegraph/client/RestClient.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/main/java/org/apache/hugegraph/driver/HugeClient.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/main/java/org/apache/hugegraph/driver/HugeClient.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/main/java/org/apache/hugegraph/exception/ServerException.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/api/auth/UserApiTest.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/api/auth/UserApiTest.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/api/auth/UserApiTest.java
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/main/java/org/apache/hugegraph/driver/HugeClient.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/main/java/org/apache/hugegraph/driver/HugeClient.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Outdated
Show resolved
Hide resolved
hugegraph-client/src/test/java/org/apache/hugegraph/functional/EdgeTest.java
Outdated
Show resolved
Hide resolved
.github/workflows/hubble-ci.yml
Outdated
@@ -23,7 +24,7 @@ on: | |||
env: | |||
TRAVIS_DIR: hugegraph-hubble/hubble-dist/assembly/travis | |||
# TODO: need update it later (eed6103359fe40d2f1476fb8c56d9388c3111a99) | |||
COMMIT_ID: be6ee386b9939dc6bd6fcbdf2274b8acc3a0a314 | |||
COMMIT_ID: 69e6b461add1051831dd6cc0c36a5e249a3b3176 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@imbajin can we share the same COMMIT_ID with client-ci.yml (just use an global env var)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@imbajin can we share the same COMMIT_ID with client-ci.yml (just use an global env var)
I do want it too but not found the way to achieve it, however I get a better way to avoid hard code commit ID in ci
we could use "latestCommitID - X" (X >= 15 maybe) to get the stable code in server & no need update the ID by ourselves if not necessary (will submit a PR after this adaptor) #542
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a method to use latestCommitID - X
, but we can't control it freely. If we want to adapt to the latest code, we still need to change the X.
we may try reusable-workflow: https://docs.github.com/en/actions/using-workflows/reusing-workflows#using-inputs-and-secrets-in-a-reusable-workflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a method to use
latestCommitID - X
, but we can't control it freely. If we want to adapt to the latest code, we still need to change the X. we may try reusable-workflow: docs.github.com/en/actions/using-workflows/reusing-workflows#using-inputs-and-secrets-in-a-reusable-workflow
Get it, but we can't set secrets/variables
in settings, we lack the permission to set it 😢 (This is also the reason why in the early code, we could only write fixed commitID in multiple CI files)
BTW, we could create/submit a common action
and publish it to github action repo & let other actions to extend it if github supports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Purpose of the PR
之前提过一个draft PR(已关闭):
直接在client中将jersey相关代码用okhttp替换掉。
对应的issue见:
后面将对应的实现在common中进行实现,并提交了PR(已合入主分支):
所以现在需要对toochain的代码进行整改,升级common版本,并进行代码适配,移除jersey相关的代码
Main Changes
Verifying these changes
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODO
Doc - Done
Doc - No Need